Skip to content

keyboard: put selected layout first and seed KDE skel config - #226

Open
ptr1337 wants to merge 1 commit into
cachyosfrom
keyboard2
Open

keyboard: put selected layout first and seed KDE skel config#226
ptr1337 wants to merge 1 commit into
cachyosfrom
keyboard2

Conversation

@ptr1337

@ptr1337 ptr1337 commented May 24, 2026

Copy link
Copy Markdown
Member

Use the user's chosen layout as XKB group 1 everywhere (live apply, locale1, KWin, and install-time files) so non-ASCII layouts default correctly with US as the fallback. Refresh additional layout info and group-switch options in createJobs(), and write etc/skel/.config/kxkbrc during install when configure.kwin is enabled.

Use the user's chosen layout as XKB group 1 everywhere (live apply,
locale1, KWin, and install-time files) so non-ASCII layouts default
correctly with US as the fallback. Refresh additional layout info and
group-switch options in createJobs(), and write etc/skel/.config/kxkbrc
during install when configure.kwin is enabled.

Signed-off-by: Peter Jung <admin@ptr1337.dev>
@ptr1337
ptr1337 requested a review from Copilot May 24, 2026 18:26
@ptr1337 ptr1337 self-assigned this May 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts keyboard layout handling so the user-selected layout is consistently treated as XKB group 1 (with an additional fallback layout as group 2) across live application, locale1, KWin, and install-time written config. It also adds install-time seeding of KDE’s kxkbrc into /etc/skel when KWin configuration is enabled.

Changes:

  • Reorders layout/variant lists so the selected layout/variant is written first everywhere (X11, /etc/default/keyboard, locale1, KWin).
  • Refreshes AdditionalLayoutInfo and group-switch options in Config::createJobs().
  • Adds optional writing of KDE Plasma keyboard config to etc/skel/.config/kxkbrc during install, plus updates unit tests for the new variant ordering.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/modules/keyboard/Tests.cpp Updates expected variant list ordering in tests.
src/modules/keyboard/SetKeyboardLayoutJob.h Extends job API with a flag to write KDE keyboard config.
src/modules/keyboard/SetKeyboardLayoutJob.cpp Reorders written layouts/variants and adds /etc/skel/.config/kxkbrc generation.
src/modules/keyboard/Config.cpp Reorders layout/variant application for XKB/locale1/KWin and refreshes additional layout info before creating jobs.
Comments suppressed due to low confidence (2)

src/modules/keyboard/SetKeyboardLayoutJob.cpp:483

  • The KDE skel seeding is skipped entirely if the target doesn't already have etc/skel (skelDir.exists()). Since writeKdeKeyboardData() already creates the parent directory for kxkbrc, consider creating etc/skel as needed (mkpath) or at least logging a warning; otherwise enabling writeKdeKeyboardConfig may silently do nothing on systems without a pre-existing /etc/skel.
    if ( m_writeKdeKeyboardConfig && !( m_skipIfNoRoot && ( destDir.isEmpty() || destDir.isRoot() ) ) )
    {
        QDir skelDir( destDir.absoluteFilePath( "etc/skel" ) );
        if ( skelDir.exists() )
        {
            QString kdeKeyboardPath = skelDir.absoluteFilePath( ".config/kxkbrc" );
            if ( !writeKdeKeyboardData( kdeKeyboardPath, m_model, m_layout, m_variant, m_additionalLayoutInfo ) )
            {
                return Calamares::JobResult::error(
                    tr( "Failed to write keyboard configuration for KDE Plasma.", "@error" ),
                    tr( "Failed to write to %1", "@error, %1 is KDE keyboard configuration path" ).arg( kdeKeyboardPath ) );
            }
        }
    }

src/modules/keyboard/SetKeyboardLayoutJob.cpp:472

  • The skip condition uses QDir::isEmpty() ("destDir.isEmpty()"), but QDir::isEmpty() checks whether the directory contains entries, not whether the QDir was constructed from an empty path / missing rootMountPoint. If the intent is to detect an unset rootMountPoint, use the rootMountPoint string’s isEmpty() (or destDir.path().isEmpty()) instead; otherwise this condition can behave unexpectedly depending on the current working directory’s contents.

    if ( m_writeKdeKeyboardConfig && !( m_skipIfNoRoot && ( destDir.isEmpty() || destDir.isRoot() ) ) )
    {
        QDir skelDir( destDir.absoluteFilePath( "etc/skel" ) );

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

{
config.setValue( QStringLiteral( "ResetOldOptions" ), true );
config.setValue( QStringLiteral( "Options" ), additionalLayoutInfo.groupSwitcher );
}

@vnepogodin vnepogodin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rebase on cachyos branch missing vconsole.conf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants